[NFC][lld][ELF] Allow customized reproduce path #151656
Conversation
Use std::optional<std::string> as return type of getReproduceOption instead of const char *. This is same as getReproduceFile in COFF which allows user to modify reproduce tar name.
|
@llvm/pr-subscribers-lld-elf @llvm/pr-subscribers-lld Author: Haohai Wen (HaohaiWen) ChangesUse std::optional<std::string> as return type of getReproduceOption Full diff: https://github.com/llvm/llvm-project/pull/151656.diff 1 Files Affected:
diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index 6c2f318ffe469..1a33fdc21b25c 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -482,10 +482,14 @@ static void checkOptions(Ctx &ctx) {
ErrAlways(ctx) << "-z force-ibt may not be used with -z retpolineplt";
}
-static const char *getReproduceOption(opt::InputArgList &args) {
+static std::optional<std::string> getReproduceOption(opt::InputArgList &args) {
if (auto *arg = args.getLastArg(OPT_reproduce))
return arg->getValue();
- return getenv("LLD_REPRODUCE");
+
+ if (const char *env = getenv("LLD_REPRODUCE"))
+ return env;
+
+ return std::nullopt;
}
static bool hasZOption(opt::InputArgList &args, StringRef key) {
@@ -681,11 +685,11 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
if (args.hasArg(OPT_v) || args.hasArg(OPT_version))
Msg(ctx) << getLLDVersion() << " (compatible with GNU linkers)";
- if (const char *path = getReproduceOption(args)) {
+ if (std::optional<std::string> path = getReproduceOption(args)) {
// Note that --reproduce is a debug option so you can ignore it
// if you are trying to understand the whole picture of the code.
Expected<std::unique_ptr<TarWriter>> errOrWriter =
- TarWriter::create(path, path::stem(path));
+ TarWriter::create(*path, path::stem(*path));
if (errOrWriter) {
ctx.tar = std::move(*errOrWriter);
ctx.tar->append("response.txt", createResponseFile(args));
|
|
I don't understand the change. The ELF port already respects |
We'd like linker to auto create tar name based on output instead of manually specify a different reproduce flag every time. That requires to store its temporary name. (e.g. when user exports LLD_REPRODUCE_DIR, lld will create {output}.tar in this folder) I don't know whether community is interested in that. I know this PR looks weird for unnecessary copy. If that is not acceptable, I can defer upstreaming this change until we really need to create a temporary name. |
|
Let's defer it until real temporary name storage is required. |
|
I don't think the proposed LLD_REPRODUCE_DIR is necessary. You can always a wrapper to add a --reproduce= option. The current reproduce file behavior is simple and easy to explain. |
3 participants
Use
std::optional<std::string>as return type of getReproduceOptioninstead of
const char *. This is same as getReproduceFile in COFF whichallows linker to modify reproduce tar name.